Skip to content

[pull] master from git:master#202

Merged
pull[bot] merged 60 commits into
turkdevops:masterfrom
git:master
May 11, 2026
Merged

[pull] master from git:master#202
pull[bot] merged 60 commits into
turkdevops:masterfrom
git:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull Bot commented May 11, 2026

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

jnavila and others added 30 commits April 6, 2026 09:38
     * convert commands to synopsis style
     * use _<placeholder>_ for arguments
     * fix conditional text to sentence limits

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
     * convert commands and options to synopsis style
     * use _<placeholder>_ for arguments
     * small style fixes

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
     * convert commands and options to synopsis style
     * use _<placeholder>_ for arguments
     * small style fixes

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
     * convert commands and options to synopsis style
     * use _<placeholder>_ for arguments

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
…alid

The negation in front of the object existence check in
cache_tree_fully_valid() was lost in 062b914 (treewide: convert
users of `repo_has_object_file()` to `has_object()`, 2025-04-29),
turning `!repo_has_object_file(...)` into `has_object(...)` instead
of `!has_object(...)`.

This makes cache_tree_fully_valid() always report the cache tree as
invalid when objects exist (the common case), forcing callers like
write_index_as_tree() to call cache_tree_update() on every
invocation.  An odb_has_object() check inside update_one() avoids a
full tree rebuild, but the unnecessary call still pays the cost of
opening an ODB transaction and, in partial clones, a promisor remote
check.

Restore the missing negation and add a test that verifies write-tree
takes the cache-tree shortcut when the cache tree is valid.

Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: David Lin <davidlin@stripe.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a server advertises promisor remotes and the client accepts some
of them, those remotes carry the server's intent: 'fetch missing
objects preferably from here', and the client agrees with that for the
remotes it accepts.

However promisor_remote_get_direct() actually iterates over all
promisor remotes in list order, which is the order they appear in the
config files (except perhaps for the one appearing in the
`extensions.partialClone` config variable which is tried last).

This means an existing, but not accepted, promisor remote, could be
tried before the accepted ones, which does not reflect the intent of
the agreement between client and server.

If the client doesn't care about what the server suggests, it should
accept nothing and rely on its remotes as they are already configured.

To better reflect the agreement between client and server, let's make
promisor_remote_get_direct() try the accepted promisor remotes before
the non-accepted ones.

Concretely, let's extract a try_promisor_remotes() helper and call it
twice from promisor_remote_get_direct():

- first with an `accepted_only=true` argument to try only the accepted
  remotes,
- then with `accepted_only=false` to fall back to any remaining remote.

Ensuring that accepted remotes are preferred will be even more
important if in the future a mechanism is developed to allow the
client to auto-configure remotes that the server advertises. This will
in particular avoid fetching from the server (which is already
configured as a promisor remote) before trying the auto-configured
remotes, as these new remotes would likely appear at the end of the
config file, and as the server might not appear in the
`extensions.partialClone` config variable.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `in_list == 0` path of all_fields_match() looks up the remote in
`config_info` by `advertised->name` repeatedly, even though every
caller in should_accept_remote() has already performed this
lookup and holds the result in `p`.

To avoid this useless work, let's replace the `int in_list`
parameter with a `struct promisor_info *config_entry` pointer:

 - When NULL (ACCEPT_ALL mode): scan the whole `config_info` list, as
   the old `in_list == 1` path did.

 - When non-NULL: match against that single config entry directly,
   avoiding the redundant string_list_lookup() call.

This removes the hidden dependency on `advertised->name` inside
all_fields_match(), which would be wrong if in the future
auto-configured remotes are implemented, as the local config name may
differ from the server's advertised name.

While at it, let's also add a comment before all_fields_match() and
match_field_against_config() to help understand how things work and
help avoid similar issues.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In should_accept_remote() and parse_one_advertised_remote(), when a
remote is ignored, we tell users why it is ignored in a warning, but we
don't tell them that the remote is actually ignored.

Let's clarify that, so users have a better idea of what's actually
happening.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In parse_one_advertised_remote(), we check for a NULL remote name and
remote URL, but not for empty ones. An empty URL seems possible as
url_percent_decode("") doesn't return NULL.

In promisor_config_info_list(), we ignore remotes with empty URLs, so a
Git server should not advertise remotes with empty URLs. It's possible
that a buggy or malicious server would do it though.

So let's tighten the check in parse_one_advertised_remote() to also
reject empty strings at parse time.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A previous commit made sure we now reject empty URLs early at parse
time. This makes the existing warning() in case a remote URL is NULL
or empty very unlikely to be useful.

In future work, we also plan to add URL-based acceptance logic into
should_accept_remote().

To adapt to previous changes and prepare for upcoming changes, let's
restructure the control flow in should_accept_remote().

Concretely, let's:

 - Replace the warning() in case of an empty URL with a BUG(), as a
   previous commit made sure empty URLs are rejected early at parse
   time.

 - Move that modified empty-URL check to the very top of the function,
   so that every acceptance mode, instead of only ACCEPT_KNOWN_URL, is
   covered.

 - Invert the URL comparison: instead of returning on match and
   warning on mismatch, return early on mismatch and let the match
   case fall through. This opens a single exit path at the bottom of
   the function for future commits to extend.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a future commit we are going to check if some strings contain
control characters, so let's refactor the logic to do that in a new
has_control_char() helper function.

It cleans up the code a bit anyway.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In future commits, we are going to add more logic to
filter_promisor_remote() which is already doing a lot of things.

Let's alleviate that by moving the logic that checks and validates the
value of the `promisor.acceptFromServer` config variable into its own
accept_from_server() helper function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In filter_promisor_remote(), the instances of `struct promisor_info`
for accepted remotes are dismantled into separate parallel data
structures (the 'accepted' strvec for server names, and
'accepted_filters' for filter strings) and then immediately freed.

Instead, let's keep these instances on an 'accepted_remotes' list.

This way the post-loop phase can iterate a single list to build the
protocol reply, apply advertised filters, and mark remotes as
accepted, rather than iterating three separate structures.

This refactoring also prepares for a future commit that will add a
'local_name' member to 'struct promisor_info'. Since struct instances
stay alive, downstream code will be able to simply read both names
from them rather than needing yet another parallel strvec.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a previous commit, filter_promisor_remote() was refactored to keep
accepted 'struct promisor_info' instances alive instead of dismantling
them into separate parallel data structures.

Let's go one step further and replace the 'struct strvec *accepted'
argument passed to filter_promisor_remote() with a
'struct string_list *accepted_remotes' argument.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In t5710, we frequently construct local file URIs using `file://$(pwd)`.
On Unix-like systems, $(pwd) returns an absolute path starting with a
slash (e.g., `/tmp/repo`), resulting in a valid 3-slash URI with an
empty host (`file:///tmp/repo`).

However, on Windows, $(pwd) returns a path starting with a drive
letter (e.g., `D:/a/repo`). This results in a 2-slash URI
(`file://D:/a/repo`). Standard URI parsers misinterpret this format,
treating `D:` as the host rather than part of the absolute path.

This is to be expected because RFC 8089 says that the `//` prefix with
an empty local host must be followed by an absolute path starting with
a slash.

While this hasn't broken the existing tests (because the old
`promisor.acceptFromServer` logic relies entirely on strict `strcmp()`
without normalizing the URLs), it will break future commits that pass
these URLs through `url_normalize()` or similar functions.

To future-proof the tests and ensure cross-platform URI compliance,
let's introduce a $TRASH_DIRECTORY_URL helper variable that explicitly
guarantees a leading slash for the path component, ensuring valid
3-slash `file:///` URIs on all operating systems.

While at it, let's also introduce $ENCODED_TRASH_DIRECTORY_URL to
handle some common special characters in directory paths.

To be extra safe, let's skip all the tests if there are uncommon
special characters in the directory path.

Then let's replace all instances of `file://$(pwd)` with
$TRASH_DIRECTORY_URL across the test script, and let's simplify the
`sendFields` and `checkFields` tests to use
$ENCODED_TRASH_DIRECTORY_URL directly.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_files_ref_lock_timeout_ms() calls repo_config_get_int() using
the_repository, as no repository instance is available in its scope. Add a
struct repository parameter and use it instead of the_repository.

Update all callers accordingly. In files-backend.c, lock_raw_ref() can
obtain repository instance from the struct ref_transaction via
transaction->ref_store->repo and pass it down. For create_reflock(), which
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.

This reduces reliance on the_repository global, though the function
still uses static variables and is not yet fully repository-scoped.
This can be addressed in a follow-up change.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs.c uses the_hash_algo in multiple places, relying on global state for
the object hash algorithm. Replace these uses with the appropriate
repository-specific hash_algo. In transaction-related functions
(ref_transaction_create, ref_transaction_delete, migrate_one_ref, and
transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo.
In other cases, such as repo_get_submodule_ref_store(), use
repo->hash_algo.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable_be_init() and reftable_be_create_on_disk() use the_repository even
though a repository instance is already available, either directly or via
struct ref_store.

Replace these uses with the appropriate local repository instance (repo or
ref_store->repo) to avoid relying on global state.

Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
is_bare_repository() is still there in the file.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We missed the cut-off for Rust by default in 2.53, but we still can
enable it by default for 2.54, so update our breaking changes document
accordingly.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We'll make Rust the default in a future commit, so be sure to install
Cargo (which will also install Rust) to prepare for that case.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Older versions of Rust on Linux, such as that used in Debian 11 in our
CI, require linking against libdl.  Were we linking with Cargo, this
would be included automatically, but since we're not, explicitly set it
in the system-specific config.

This library is part of libc, so linking against it if it happens to be
unnecessary will add no dependencies to the resulting binary.  In
addition, it is provided by both glibc and musl, so it should be
portable to almost all Linux systems.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our breaking changes document says that we'll enable Rust by default in
Git 2.54.  Adjust the Makefile to switch the option from WITH_RUST to
NO_RUST to enable it by default and update the help text accordingly.
Similarly, for Meson, enable the option by default and do not
automatically disable it if Cargo is missing, since the goal is to help
users find where they are likely to have problems in the future.

Update our CI tests to swap out the single Linux job with Rust to a
single job without, both for Makefile and Meson.  Similarly, update the
Windows Makefile job to not use Rust, while the Meson job (which does
not build with ci/lib.sh) will default to having it enabled.

Move the check for Cargo in the Meson build because it is no longer
needed in the main script.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is an old pre-existing memory leak in repo_init() due to failing
to call clear_repository_format() in the error case.

It went undetected because a specific bug is required to trigger it:
enable a v1 extension in a repository with format v0. Obviously this
can only happen in a development environment, so it does not trigger
in normal usage, however the memleak is real and needs fixing.

Fix it by also calling clear_repository_format() in the error case.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Next commits add a 'hook.jobs' config option of type 'unsigned int',
so add a helper to parse it since the API only supports int and ulong.

An alternative is to make 'hook.jobs' an 'int' or parse it as an 'int'
then cast it to unsigned, however it's better to use proper helpers for
the type. Using 'ulong' is another option which already has helpers, but
it's a bit excessive in size for just the jobs number.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The hook.jobs config is a global way to set hook parallelization for
all hooks, in the sense that it is not per-event nor per-hook.

Finer-grained configs will be added in later commits which can override
it, for e.g. via a per-event type job options. Next commits will also
add to this item's documentation.

Parse hook.jobs config key in hook_config_lookup_all() and store its
value in hook_all_config_cb.jobs, then transfer it into r->jobs after
the config pass completes.

This is mostly plumbing and the cached value is not yet used.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hooks always run in sequential order due to the hardcoded jobs == 1
passed to run_process_parallel(). Remove that hardcoding to allow
users to run hooks in parallel (opt-in).

Users need to decide which hooks to run in parallel, by specifying
"parallel = true" in the config, because Git cannot know if their
specific hooks are safe to run or not in parallel (for e.g. two hooks
might write to the same file or call the same program).

Some hooks are unsafe to run in parallel by design: these will marked
in the next commit using RUN_HOOKS_OPT_INIT_FORCE_SERIAL.

The hook.jobs config specifies the default number of jobs applied to all
hooks which have parallelism enabled.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pre-push is the only hook that keeps stdout and stderr separate (for
backwards compatibility with git-lfs and potentially other users). This
prevents parallelizing it because run-command needs stdout_to_stderr=1
to buffer and de-interleave parallel outputs.

Since we now default to jobs=1, backwards compatibility is maintained
without needing any extension or extra config: when no parallelism is
requested, pre-push behaves exactly as before.

When the user explicitly opts into parallelism via hook.jobs > 1,
hook.<event>.jobs > 1, or -jN, they accept the changed output behavior.

Document this and let get_hook_jobs() set stdout_to_stderr=1 automatically
when jobs > 1, removing the need for any extension infrastructure.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Several hooks are known to be inherently non-parallelizable, so initialize
them with RUN_HOOKS_OPT_INIT_FORCE_SERIAL. This pins jobs=1 and overrides
any hook.jobs or runtime -j flags.

These hooks are:
applypatch-msg, pre-commit, prepare-commit-msg, commit-msg, post-commit,
post-checkout, and push-to-checkout.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Expose the parallel job count as a command-line flag so callers can
request parallelism without relying only on the hook.jobs config.

Add tests covering serial/parallel execution and TTY behaviour under
-j1 vs -jN.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a hook.<event>.jobs count config that allows users to override the
global hook.jobs setting for specific hook events.

This allows finer-grained control over parallelism on a per-event basis.

For example, to run `post-receive` hooks with up to 4 parallel jobs
while keeping other events at their global default:

[hook]
    post-receive.jobs = 4

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10ne1 and others added 27 commits April 10, 2026 07:58
Allow -1 as a value for hook.jobs, hook.<event>.jobs, and the -j
CLI flag to mean "use as many jobs as there are CPU cores", matching
the convention used by fetch.parallel and other Git subsystems.

The value is resolved to online_cpus() at parse time so the rest
of the code always works with a positive resolved count.

Other non-positive values (0, -2, etc) are rejected with a warning
(config) or die (CLI).

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We recently fixed a bug in commit 2226ffa (run_processes_parallel():
fix order of sigpipe handling, 2026-04-08) where a hook that caused us
to get SIGPIPE would accidentally trigger the run_processes_parallel()
cleanup handler killing the child processes.

For a single hook, this meant killing the already-exited hook. This case
was triggered by our tests, but was only a problem on some platforms.

But if you have multiple hooks running in parallel, this causes a
problem everywhere, since one hook failing to read its input would take
down all hooks. Now that we have parallel hook support, we can add a
test for this case. It should pass already, due to the existing fix.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The scheme driver separates identifiers only at parentheses of all
sorts and whitespace, except that vertical bars act as brackets that
enclose an identifier.

The test case attempts to demonstrate the vertical bars with a change
from 'some-text' to '|a greeting|'. However, this misses the goal
because the same word coloring would be applied if '|a greeting|'
were parsed as two words.

Have an identifier between vertical bars with a space in both the pre-
and the post-image and change only one side of the space to show that
the single word exists between the vertical bars.

Also add cases that change parentheses of all kinds in a sequence of
parentheses to show that they are their own word each.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Scott L. Burson <Scott@sympoiesis.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Common Lisp has top-level forms, such as 'defun' and 'defmacro', that
are not matched by the current Scheme pattern.  Also, it is more
common in CL, when defining user macros intended as top-level forms,
to prefix their names with "def" instead of "define"; such forms are
also not matched.  And some top-level forms don't even begin with
"def".

On the other hand, it is an established formatting convention in the
Lisp community that only top-level forms start at the left margin.  So
matching any unindented line starting with an open parenthesis is an
acceptable heuristic; false positives will be rare.

However, there are also cases where notionally top-level forms are
grouped together within some containing form.  At least in the Common
Lisp community, it is conventional to indent these by two spaces, or
sometimes one.  But matching just an open parenthesis indented by two
spaces would be too broad; so the pattern added by this commit
requires an indented form to start with "(def".  It is believed that
this strikes a good balance between potential false positives and
false negatives.

Signed-off-by: Scott L. Burson <Scott@sympoiesis.com>
Acked-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Most unfortunately macOS does not support st_[amc]tim for timestamps
down to nanosecond resolution as POSIX systems.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have a couple of calls to `test_match_signal ()` where we execute a
Git command and expect it to die with a specific signal. These calls
will essentially execute the process in a subshell via `foo; echo $?`,
but as we expect `foo` to fail this will cause the overall subshell to
fail once we `set -e`.

Fix this issue by using `foo && echo 0 || echo $?` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The helper function `test_must_fail ()` executes a specific Git command
that may or may not fail in a specific way. This is done by executing
the command in question and then comparing its exit code against a set
of conditions.

This works, but once we run our test suite with `set -e` we may bail out
of `test_must_fail ()` early in case the command actually fails, even
though we expect it to fail. Prepare for this change by handling the
failed case with `||`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have a couple of calls to `stop_git_daemon ()` outside of specific
test cases that will kill a backgrounded git-daemon(1) process and
expect the process with a specific error code. While these function
calls do end up killing git-daemon(1), the error handling we have in
those contexts is basically ineffective. So while we expect the process
to exit with a specific error code, we will just continue with any error
in case it doesn't.

This will change once we enable `set -e` in a subsequent commit. There's
two issues though that will make this _always_ fail:

  - Our call to `wait` is expected to fail, but because it's not part of
    a condition it will cause us to bail out immediately with `set -e`.

  - We try to kill git-daemon(1) a second time via the pidfile. We can
    generally expect that this is the same PID though as we had in the
    "GIT_DAEMON_PID" environment variable, and thus it's more likely
    than not that we have already killed it, and the call to kill will
    fail.

Prepare for this change by handling the failure of `wait` with `||` and
by silencing failures of the second call to `kill`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have a couple of calls to `git config --unset` that ultimately end up
as no-ops as the configuration variables aren't set (anymore) in the
first place. These calls are mostly intended to recover unconditionally
from tests that may have executed only partially, but they'll ultimately
fail during a normal test run.

This hasn't been a problem until now as we aren't running tests with
`set -e`. This is about to change though, so let's silence the case
where we cannot unset the config keys.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have some test in our test suite where we use the pattern of
`test ... && test_expect_succeess` to conditionally execute a test. The
problem is that when we decide to not execute the test, we'll indeed
skip the test, but the overall statement will also be unsuccessful. This
will become a problem once we enable `set -e`.

Prepare for this future by turning this into a proper conditional, which
is also a bit easier to read overall.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Several of our tests verify whether a certain binary can be executed,
potentially skipping tests in case we cannot, for example because the
binary doesn't exist. In those cases we often run the binary outside of
any conditionally.

This will start to fail once we enable `set -e`, as that will cause us
to bail out the test immediately. Improve these tests by executing them
inside of a conditional instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Both `test_when_finished ()` and `test_atexit ()` build up a chain of
cleanup commands by prepending each new command to the existing cleanup
string. To preserve the exit code of the test body across cleanup
execution, we append the following logic:

    } && (exit "$eval_ret"); eval_ret=$?; ...

The intent of this is to run the cleanup block and then unconditionally
restore `eval_ret`. The original behaviour of this is is:

   +------------------+---------+------------------------------------+
   |test body         │ cleanup │ old behaviour                      │
   +------------------+---------+------------------------------------+
   │pass (eval_ret=0) | pass    │ && taken -> (exit 0) -> eval_ret=0 |
   +------------------+---------+------------------------------------+
   │pass (eval_ret=0) | fail    │ && not taken -> eval_ret=$?        |
   +------------------+---------+------------------------------------+
   │fail (eval_ret=1) | pass    │ && taken -> (exit 1) -> eval_ret=1 |
   +------------------+---------+------------------------------------+
   │fail (eval_ret=1) | fail    | && not taken -> eval_ret=$?        |
   +------------------+---------+------------------------------------+

This logic will start to fail once we enable `set -e`. When `$eval_ret`
is non-zero, the subshell we create will fail, and with `set -e` we'll
thus bail out without evaluating the logic after the semicolon.

Fix this issue by instead using `|| eval_ret=\$?; ...`. Besides being
a bit simpler, it also retains the original behaviour:

   +------------------+---------+------------------------------------+
   |test body         │ cleanup │ old behaviour                      │
   +------------------+---------+------------------------------------+
   │pass (eval_ret=0) | pass    │ || not taken -> eval_ret unchanged |
   +------------------+---------+------------------------------------+
   │pass (eval_ret=0) | fail    │ || taken -> eval_ret=$?            |
   +------------------+---------+------------------------------------+
   │fail (eval_ret=1) | pass    │ || not taken -> eval_ret unchanged |
   +------------------+---------+------------------------------------+
   │fail (eval_ret=1) | fail    | || taken -> eval_ret=$?            |
   +------------------+---------+------------------------------------+

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In t0008 we use `grep -v` in a subshell, but expect that this command
will sometimes not match anything. This would cause grep(1) to return an
error code, but given that we don't run with `set -e` we swallow this
error.

We're about to enable `set -e`. Prepare for this by ignoring any errors.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In t1301 we're trying to remove any potentially-existing default ACLs
that might exist on the transh directory by executing setfacl(1).
According to 8ed0a74 (t1301-shared-repo.sh: don't let a default ACL
interfere with the test, 2008-10-16), this is done because we play
around with permissions and umasks in this test suite.

The setfacl(1) binary may not exist on some systems though, even though
tests ultimately still pass. This doesn't matter currently, but will
cause the test to fail once we start running with `set -e`. Silence such
failures by ignoring failures here.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In `test_bisection_diff ()` we use `expr` to perform some math. This
command has some gotchas though in that it will only return success when
the result is neither null nor zero. In some of our cases though it
actually _is_ zero, and that will cause the expressions to fail once we
enable `set -e`.

Prepare for this change by instead using `$(( ))`, which doesn't have
the same issue. While at it, modernize the function a tiny bit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In t9902 we're using the `read` builtin to read some values into a
variable. This is done by using `-d ""`, which cause us to read until
the end of the heredoc. As the read is terminated by EOF, the command
will end up returning a non-zero error code. This hasn't been an issue
until now as we didn't run with `set -e`, but that'll change in a
subsequent commit.

Prepare for this change by not using read at all, as we can simply store
the multi-line value directly.

Suggested-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have recently merged a patch series that had a simple misspelling of
`test_expect_success`. Instead of making our tests fail though, this
typo went completely undetected and all of our tests passed, which is of
course unfortunate. This is a more general issue with our test suite:
all commands that run outside of a specific test case can fail, and if
we don't explicitly check for such failure then this failure will be
silently ignored.

Improve the status quo by enabling the errexit option so that any such
unchecked failures will cause us to abort immediately.

Note that for now, we only enable this option for Bash 5 and newer. This
is because other shells have wildly different behaviour, and older
versions of Bash (especially on macOS) are buggy. The list of enabled
shells may be extended going forward.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up to use the right instance of a repository instance in
calls inside refs subsystem.

* sp/refs-reduce-the-repository:
  refs/reftable-backend: drop uses of the_repository
  refs: remove the_hash_algo global state
  refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
Doc mark-up updates.

* ja/doc-difftool-synopsis-style:
  doc: convert git-describe manual page to synopsis style
  doc: convert git-shortlog manual page to synopsis style
  doc: convert git-range-diff manual page to synopsis style
  doc: convert git-difftool manual page to synopsis style
The check that implements the logic to see if an in-core cache-tree
is fully ready to write out a tree object was broken, which has
been corrected.

* dl/cache-tree-fully-valid-fix:
  cache-tree: fix inverted object existence check in cache_tree_fully_valid
Promisor remote handling has been refactored and fixed in
preparation for auto-configuration of advertised remotes.

* cc/promisor-auto-config-url:
  t5710: use proper file:// URIs for absolute paths
  promisor-remote: remove the 'accepted' strvec
  promisor-remote: keep accepted promisor_info structs alive
  promisor-remote: refactor accept_from_server()
  promisor-remote: refactor has_control_char()
  promisor-remote: refactor should_accept_remote() control flow
  promisor-remote: reject empty name or URL in advertised remote
  promisor-remote: clarify that a remote is ignored
  promisor-remote: pass config entry to all_fields_match() directly
  promisor-remote: try accepted remotes before others in get_direct()
Hook scripts defined via the configuration system can now be
configured to run in parallel.

* ar/parallel-hooks:
  t1800: test SIGPIPE with parallel hooks
  hook: allow hook.jobs=-1 to use all available CPU cores
  hook: add hook.<event>.enabled switch
  hook: move is_known_hook() to hook.c for wider use
  hook: warn when hook.<friendly-name>.jobs is set
  hook: add per-event jobs config
  hook: add -j/--jobs option to git hook run
  hook: mark non-parallelizable hooks
  hook: allow pre-push parallel execution
  hook: allow parallel hook execution
  hook: parse the hook.jobs config
  config: add a repo_config_get_uint() helper
  repository: fix repo_init() memleak due to missing _clear()
Doc update.

* jc/doc-timestamps-in-stat:
  CodingGuidelines: st_mtimespec vs st_mtim vs st_mtime
The userdiff driver for the Scheme language has been extended to
cover other Lisp dialects.

* sb/userdiff-lisp-family:
  userdiff: extend Scheme support to cover other Lisp dialects
  userdiff: tighten word-diff test case of the scheme driver
Rust support is enabled by default (but still allows opting out) in
some future version of Git.

* bc/rust-by-default:
  Enable Rust by default
  Linux: link against libdl
  ci: install cargo on Alpine
  docs: update version with default Rust support
The test suite harness and many individual test scripts have been
updated to work correctly when 'set -e' is in effect, which helps
detect misspelled test commands.

* ps/test-set-e-clean:
  t: detect errors outside of test cases
  t9902: fix use of `read` with `set -e`
  t6002: fix use of `expr` with `set -e`
  t1301: don't fail in case setfacl(1) doesn't exist or fails
  t0008: silence error in subshell when using `grep -v`
  t: prepare `test_when_finished ()`/`test_atexit()` for `set -e`
  t: prepare execution of potentially failing commands for `set -e`
  t: prepare conditional test execution for `set -e`
  t: prepare `git config --unset` calls for `set -e`
  t: prepare `stop_git_daemon ()` for `set -e`
  t: prepare `test_must_fail ()` for `set -e`
  t: prepare `test_match_signal ()` calls for `set -e`
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@pull pull Bot locked and limited conversation to collaborators May 11, 2026
@pull pull Bot added the ⤵️ pull label May 11, 2026
@pull pull Bot merged commit 8a10133 into turkdevops:master May 11, 2026
2 of 3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.